Automatic update of `RCT-Folly` #32659

fortmarek · 2021-11-25T09:31:21Z

Summary

When upgrading react-native, the version RCT-Folly defined here can change. If that happens, if you run pod install after yarn install, you will get a similar error to this:

[!] CocoaPods could not find compatible versions for pod "RCT-Folly":
  In snapshot (Podfile.lock):
    RCT-Folly (from `../node_modules/react-native/third-party-podspecs/RCT-Folly.podspec`)

  In Podfile:
    RCT-Folly (from `../node_modules/react-native/third-party-podspecs/RCT-Folly.podspec`)

    React-RCTNetwork (from `../node_modules/react-native/Libraries/Network`) was resolved to 0.66.3, which depends on
      RCT-Folly (= 2021.06.28.00-v2)

This error occurs because Cocoapods does not update pods that point to a local podspec. Locally, you could resolve this issue by running pod update RCT-Folly --no-repo-update. On the CI, you have to do a clean checkout (in case you cache the Pods folder which we do). All of this makes upgrading react-native painful - for the whole community and for us @Shopify

There are other users who have struggled with this, such as here. The suggestion there is to delete Podfile.lock which is unnecessary - but it shows that users are confused what to do with this error and is something worth fixing.

To mitigate these issues, react_native_pods.rb automatically marks RCT-Folly as changed in the detect_changes_with_podfile method from Pod::Lockfile if the version in node_modules/react-native/third-party-podspecs/RCT-Folly.podspec and Pods/Local Podspecs/RCT-Folly.podspec.json mismatch.

Instead of automatically updating the local podspec (in Pods/Local Podspecs directory) we could also:
a) integrate RCT-Folly as a local pod (such as React-Core and others)
b) integrate RCT-Folly as an external dependency (going through Cocoapods' centralized repository)

I don't have enough context on why RCT-Folly is bundled the way it is, so I am open to suggestions here.

Changelog

[iOS] [Fixed] - Fix pod install when RCT-Folly version has been updated.

Test Plan

I have created a repository where you can reproduce the issue. You can simply:

clone the repo (git clone https://github.com/fortmarek/react-native-upgrade-reproduction)
Run yarn install && cd ios && pod install
Change react-native version in package.json to 0.66.3
Run again yarn install && pod install
Observe error

To test the fix, you can then:

copy-paste the react_native_pods.rb file from this branch to react-native-upgrade-reproduction/node_modules/scripts/react_native_pods.rb
run pod install again

This time, the pod install command should succeed.

facebook-github-bot · 2021-11-25T09:31:25Z

Hi @fortmarek!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

analysis-bot · 2021-11-25T10:00:58Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: e330eee
Branch: main

analysis-bot · 2021-11-25T10:08:25Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,477,822	+190,304
android	hermes	armeabi-v7a	7,777,034	+157,408
android	hermes	x86	8,946,596	+188,667
android	hermes	x86_64	8,890,668	+194,370
android	jsc	arm64-v8a	9,792,621	+116,660
android	jsc	armeabi-v7a	8,753,049	+88,287
android	jsc	x86	9,741,335	+109,446
android	jsc	x86_64	10,342,161	+114,316

Base commit: e330eee
Branch: main

facebook-github-bot · 2021-11-29T09:26:59Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

facebook-github-bot · 2021-11-30T15:12:21Z

@cortinico has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

cortinico

Thanks for the PR @fortmarek that looks great,

I don't have enough context on why RCT-Folly is bundled the way it is, so I am open to suggestions here.

I'd love to get feedback from @fkgozali and @sota000 as they have more context than me on how RCT-Folly is bundled and they could spot potential problems with this PR. Overall looks good to me though 👍

fortmarek · 2021-12-08T14:51:10Z

Hey @fkgozali @sota000, would you have some time to look at this? 🙏

sota000 · 2021-12-16T02:24:55Z

Hi @fortmarek, sorry for the delayed response! My question would be that if RCT-Folly has the problem, wouldn't some other modules with the same setup have the same problem?

As for local vs. third party set up, I don't really have the history context either. It might be possible that it was done it because how the podspec file for RCT-Folly was set up. Maybe a better solution could be try to make it a local podspec?

fortmarek · 2021-12-17T10:17:53Z

Hey @sota000!

My question would be that if RCT-Folly has the problem, wouldn't some other modules with the same setup have the same problem?

Correct, I have updated the PR to automatically update all podspecs in third-party-podspecs directory. Good catch!

As for local vs. third party set up, I don't really have the history context either. It might be possible that it was done it because how the podspec file for RCT-Folly was set up. Maybe a better solution could be try to make it a local podspec?

Currently, it is a local podspec. The alternatives are:

local pod -> for this we need to have the sources locally. Unfortunately, that does not really suit us since the code for e.g. Folly is in a different repository. We could introduce git submodules to have the files locally but it's not really ideal.
remote pod -> we'd need to publish local podspecs such as Folly to the cocoapods directory. The podspecs could even keep living in the react-native repository. However, there is some maintenance tied to publishing cocoapods.

I'm leaning to keeping the current solution, although migrating to remote pods might be a good solution as well. However, that's something I cannot help with since the remote pods would have to published by Facebook employees/React Native Core group.

facebook-github-bot · 2022-01-05T10:56:01Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

fortmarek · 2022-02-04T11:19:09Z

@sota000 any updates on this one?

cortinico · 2022-02-04T23:25:02Z

@sota000 any updates on this one?

I can provide a short update. We're mostly fine with the proposed approach.
I'd like to get a final pass from @fkgozali
(sorry for this PR taking way longer than usual).

fortmarek · 2022-03-14T17:49:29Z

Hey @fkgozali, is there any chance you can look at this PR? 🙏

cipolleschi · 2022-03-17T10:31:47Z

Hi there! I was looking into this issue.
AFAIU, what's happening is that react-native has a strong dependency with RCT-Folly but there is no process that enforces that.
The proper fix should be that, every time we release a new version of react-native, we had a process which bumps the RCT-Folly version as well. This should avoid any Cocoapods black magic. Those podspecs are already complex and hard to maintain. I think we should find a better way t handle these issues, instead of adding patches to the script.

That said, for the time being, I think we can live with this patch. Improving how the pods work would be an effort that we have to carry out soon, to reduce complexity and technical debt.

react-native-bot · 2022-03-17T11:00:04Z

This pull request was successfully merged by @fortmarek in b2517c3.

^{When will my fix make it into a release? | Upcoming Releases}

Note: If you get an error like CocoaPods could not find compatible versions for pod "RCT-Folly" on `yarn` or `pod install` when crossing this commit (checking out a branch that has it from one that doesn't, or vice versa), run `pod update RCT-Folly` from `ios/` and try the command again. Background: facebook/react-native#32659 (comment) In this commit: - Bump `react` and `react-test-renderer` to follow the RN template app - Adapt to changes in Flow: - Two options removed: esproposal.optional_chaining, esproposal.nullish_coalescing - RN no longer clobbers Flow's built-in definitions for `fetch`! - Etc.; see comments - Add react-native-codegen, at ^0.0.7. The RN maintainers moved this dep back and forth between the `react-native` NPM package and the RN template app. For RN v0.65 it ended up in the template app, suggesting that projects using RN v0.65 should depend on it directly. So, do that. See discussion: zulip#5324 (comment)

Note: If you get an error like CocoaPods could not find compatible versions for pod "RCT-Folly" on `yarn` or `pod install` when crossing this commit (checking out a branch that has it from one that doesn't, or vice versa), run `pod update RCT-Folly` from `ios/` and try the command again. Background: facebook/react-native#32659 (comment) In this commit: - Bump `react` and `react-test-renderer` to follow the RN template app - Adapt to changes in Flow: - Two options removed: esproposal.optional_chaining esproposal.nullish_coalescing - RN no longer clobbers Flow's built-in definitions for `fetch`! - Etc.; see comments - Add react-native-codegen, at ^0.0.7. The RN maintainers moved this dep back and forth between the `react-native` NPM package and the RN template app. For RN v0.65 it ended up in the template app, suggesting that projects using RN v0.65 should depend on it directly. So, do that. See discussion: zulip#5324 (comment)

Note: If you get an error like CocoaPods could not find compatible versions for pod "RCT-Folly" on `yarn` or `pod install` when crossing this commit (checking out a branch that has it from one that doesn't, or vice versa), run `pod update RCT-Folly` from `ios/` and try the command again. Background: facebook/react-native#32659 (comment) In this commit: - Bump `react-native` and `flow-bin` versions - Suppress a few errors from the new Flow version (see comments) - Remove react-native-codegen, following the template-app change in facebook/react-native@ab829005b - Add a hack line to the Podfile, following the template-app change in facebook/react-native@ea5109fc0

Summary: When upgrading `react-native`, the version `RCT-Folly` defined [here](https://github.com/facebook/react-native/blob/main/third-party-podspecs/RCT-Folly.podspec) can change. If that happens, if you run `pod install` after `yarn install`, you will get a similar error to this: ``` [!] CocoaPods could not find compatible versions for pod "RCT-Folly": In snapshot (Podfile.lock): RCT-Folly (from `../node_modules/react-native/third-party-podspecs/RCT-Folly.podspec`) In Podfile: RCT-Folly (from `../node_modules/react-native/third-party-podspecs/RCT-Folly.podspec`) React-RCTNetwork (from `../node_modules/react-native/Libraries/Network`) was resolved to 0.66.3, which depends on RCT-Folly (= 2021.06.28.00-v2) ``` This error occurs because `Cocoapods` does not update pods that point to a local podspec. Locally, you could resolve this issue by running `pod update RCT-Folly --no-repo-update`. On the CI, you have to do a clean checkout (in case you cache the `Pods` folder which we do). All of this makes upgrading `react-native` painful - for the whole community and for us shopify There are other users who have struggled with this, such as [here](facebook#32423). The suggestion there is to delete `Podfile.lock` which is unnecessary - but it shows that users are confused what to do with this error and is something worth fixing. To mitigate these issues, `react_native_pods.rb` automatically marks `RCT-Folly` as changed in the [detect_changes_with_podfile method](https://github.com/CocoaPods/Core/blob/master/lib/cocoapods-core/lockfile.rb#L289) from `Pod::Lockfile` if the version in `node_modules/react-native/third-party-podspecs/RCT-Folly.podspec` and `Pods/Local Podspecs/RCT-Folly.podspec.json` mismatch. Instead of automatically updating the local podspec (in `Pods/Local Podspecs` directory) we could also: a) integrate `RCT-Folly` as a local pod (such as `React-Core` and others) b) integrate `RCT-Folly` as an external dependency (going through Cocoapods' centralized repository) I don't have enough context on why `RCT-Folly` is bundled the way it is, so I am open to suggestions here. ## Changelog  [iOS] [Fixed] - Fix `pod install` when `RCT-Folly` version has been updated. Pull Request resolved: facebook#32659 Test Plan: I have created a [repository](https://github.com/fortmarek/react-native-upgrade-reproduction) where you can reproduce the issue. You can simply: 1) clone the repo (`git clone https://github.com/fortmarek/react-native-upgrade-reproduction`) 2) Run `yarn install && cd ios && pod install` 3) Change `react-native` version in `package.json` to `0.66.3` 4) Run again `yarn install && pod install` 5) Observe error To test the fix, you can then: 1) copy-paste the `react_native_pods.rb` file from this branch to `react-native-upgrade-reproduction/node_modules/scripts/react_native_pods.rb` 2) run `pod install` again This time, the `pod install` command should succeed. Reviewed By: sota000 Differential Revision: D32720758 Pulled By: cortinico fbshipit-source-id: 940db9c9f0530f896e47b676dec46bc93cea0085

fortmarek added 3 commits November 22, 2021 11:13

Add update_folly_if_needed step in use_react_native

ca1c8c6

Do not add v2 suffix

41535e5

Fetch folly podspec instead of update

0482d42

react-native-bot added Bug Platform: iOS iOS applications. labels Nov 25, 2021

facebook-github-bot added p: Shopify Partner: Shopify Partner labels Nov 25, 2021

react-native-bot added the No CLA Authors need to sign the CLA before a PR can be reviewed. label Nov 26, 2021

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 29, 2021

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 29, 2021

react-native-bot removed the No CLA Authors need to sign the CLA before a PR can be reviewed. label Nov 29, 2021

Use patch for detect_podfile_changes to auto-update RCT-Folly

4aace98

fortmarek force-pushed the main branch from 873044a to 4aace98 Compare November 29, 2021 10:02

cortinico approved these changes Nov 30, 2021

View reviewed changes

Auto-update all necessary local podspecs

8a4b68a

fotos mentioned this pull request Feb 25, 2022

fix: add missing CACHE_VERSION in pod_install react-native-community/react-native-circleci-orb#142

Merged

cipolleschi approved these changes Mar 17, 2022

View reviewed changes

facebook-github-bot closed this in b2517c3 Mar 17, 2022

react-native-bot added the Merged This PR has been merged. label Mar 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic update of `RCT-Folly` #32659

Automatic update of `RCT-Folly` #32659

fortmarek commented Nov 25, 2021 •

edited

Loading

facebook-github-bot commented Nov 25, 2021

analysis-bot commented Nov 25, 2021 •

edited

Loading

analysis-bot commented Nov 25, 2021 •

edited

Loading

facebook-github-bot commented Nov 29, 2021

facebook-github-bot commented Nov 30, 2021

cortinico left a comment

fortmarek commented Dec 8, 2021

sota000 commented Dec 16, 2021

fortmarek commented Dec 17, 2021

facebook-github-bot commented Jan 5, 2022

fortmarek commented Feb 4, 2022

cortinico commented Feb 4, 2022

fortmarek commented Mar 14, 2022

cipolleschi commented Mar 17, 2022

react-native-bot commented Mar 17, 2022

Automatic update of RCT-Folly #32659

Automatic update of RCT-Folly #32659

Conversation

fortmarek commented Nov 25, 2021 • edited Loading

Summary

Changelog

Test Plan

facebook-github-bot commented Nov 25, 2021

Action Required

Process

analysis-bot commented Nov 25, 2021 • edited Loading

analysis-bot commented Nov 25, 2021 • edited Loading

facebook-github-bot commented Nov 29, 2021

facebook-github-bot commented Nov 30, 2021

cortinico left a comment

Choose a reason for hiding this comment

fortmarek commented Dec 8, 2021

sota000 commented Dec 16, 2021

fortmarek commented Dec 17, 2021

facebook-github-bot commented Jan 5, 2022

fortmarek commented Feb 4, 2022

cortinico commented Feb 4, 2022

fortmarek commented Mar 14, 2022

cipolleschi commented Mar 17, 2022

react-native-bot commented Mar 17, 2022

Automatic update of `RCT-Folly` #32659

Automatic update of `RCT-Folly` #32659

fortmarek commented Nov 25, 2021 •

edited

Loading

analysis-bot commented Nov 25, 2021 •

edited

Loading

analysis-bot commented Nov 25, 2021 •

edited

Loading